[Fix] Support trace-specific color sequences in Plotly Express via templates#5437
[Fix] Support trace-specific color sequences in Plotly Express via templates#5437emilykl merged 13 commits intoplotly:mainfrom
Conversation
- Check template.data.<trace_type> for marker.color or line.color before falling back to template.layout.colorway - Handle timeline special case (maps to bar trace type) - Use marker colors first, fall back to line colors if no markers found - Fixes issue plotly#5416
- Modify apply_default_cascade to check template.data.<trace_type> for marker.color or line.color - Fallback to template.layout.colorway if trace-specific colors not found - Add comprehensive tests for trace-specific color sequences - Handle timeline special case (maps to bar trace type) - Follow existing patterns for symbol_sequence and line_dash_sequence Fixes plotly#5416
- Modify apply_default_cascade to read colors from template.data.<trace_type> - Prioritize trace-specific colors over layout.colorway - Add special case for timeline constructor (maps to bar trace type) - Add comprehensive tests for trace-specific color sequences - Test trace type isolation, fallback behavior, and timeline special case
|
@antonymilne This is a great PR, thank you for the contribution! I left a few small style/clarity comments, once those are resolved I'll approve. |
- Make constructor parameter required (was optional with None default) - Refactor trace-specific color extraction to only assign when colors are found - Improve code clarity by checking for non-empty color list before assignment
- Remove redundant check that was setting color_discrete_sequence to None - The check is unnecessary since we only assign trace_specific_colors when any() is True - Fallback logic to layout.colorway and qualitative.D3 remains intact
Co-authored-by: Emily KL <4672118+emilykl@users.noreply.github.com>
|
@emilykl thanks very much for your review! I've implemented all your suggested changes exactly as you wrote them, with one small simplification where I changed I messed up the commits a bit but you can see all the changes since your last review here: https://github.com/plotly/plotly.py/pull/5437/files/74543f301be86ab121a8069a506ced9aba2a2d53..a3853d2afc113cea2fda3ab98b435e745823abfd. P.S. if you're in the mood for reviewing, I have another small PR here 😀 #5439 |
|
@antonymilne One final change needed -- in Thanks for your work on this bugfix, and for the additional PR! I will do my best to review tomorrow or early next week. |
|
Oops yes sorry, I made the change to There's a couple of failing tests but I'm pretty sure they're not my fault - I was getting similar messages in #5439. Please could you take a look? 🙏 Maybe they're just flaky and need rerunning? I'm not sure. |
|
@antonymilne Awesome! You're right, those failing tests are not related to your PR. I'll go ahead and merge. |
Summary
Fixes #5416.
You can define color sequences in a template on a per-trace basis via
template.data.<trace_type>, but these were ignored by Plotly Express. Now they are used.Changes
apply_default_cascade()to check for trace-specific colors intemplate.data.<trace_type>before falling back totemplate.layout.colorwayor default qualitative palettetest_px_templates_trace_specific_colorsExample Usage
See here for a demonstration of all (I think?) applicable trace types, i.e. those that support
marker.color.Code PR
plotly.graph_objects, my modifications concern the code generator and not the generated files.